From 541aaa23920d1e6ac27b186334aadde78a3ecf43 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 8 Jun 2020 18:47:44 +0200 Subject: [PATCH] selectionmodel: Add unselect_rest argument to select_callback This is not just about consistency with other functions. It is about avoiding reentrancy problems. GtkListBase first doing an unselect_all() will then force the SelectionModel to consider a state where all items are unselected (and potentially deciding to autoselect one) and then cause a "selection-changed" emission that unselects all items and potentially updates all the list item widgets in the GtkListBase to the unselected state. After this, GtkListBase selects new items, but to the SelectionModel and the list item widgets this looks like an enitrely new operation and there is no way to associate it with the previous state, so the SelectionModel cannot undo any previous actions it took when unselecting. And all listitem widgets will now think they were just selected and start running animations about selecting. --- gtk/gtklistbase.c | 7 +++---- gtk/gtkmultiselection.c | 13 +++++++++++-- gtk/gtkpropertyselection.c | 20 ++++++++++++++++---- gtk/gtkselectionmodel.c | 5 ++++- gtk/gtkselectionmodel.h | 2 ++ testsuite/gtk/multiselection.c | 2 +- 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/gtk/gtklistbase.c b/gtk/gtklistbase.c index 27e5075ec3..507e3c81fd 100644 --- a/gtk/gtklistbase.c +++ b/gtk/gtklistbase.c @@ -1389,10 +1389,9 @@ gtk_list_base_stop_rubberband (GtkListBase *self) } else { - if (!priv->rubberband->extend) - gtk_selection_model_unselect_all (model); - - gtk_selection_model_select_callback (model, range_cb, priv->rubberband->active); + gtk_selection_model_select_callback (model, + !priv->rubberband->extend, + range_cb, priv->rubberband->active); } g_clear_pointer (&priv->rubberband, rubberband_data_free); diff --git a/gtk/gtkmultiselection.c b/gtk/gtkmultiselection.c index a852972bee..70045b525c 100644 --- a/gtk/gtkmultiselection.c +++ b/gtk/gtkmultiselection.c @@ -175,6 +175,7 @@ gtk_multi_selection_unselect_all (GtkSelectionModel *model) static gboolean gtk_multi_selection_add_or_remove (GtkSelectionModel *model, + gboolean unselect_rest, gboolean add, GtkSelectionCallback callback, gpointer data) @@ -190,6 +191,13 @@ gtk_multi_selection_add_or_remove (GtkSelectionModel *model, min = G_MAXUINT; max = 0; + if (unselect_rest) + { + min = gtk_set_get_min (self->selected); + max = gtk_set_get_max (self->selected); + gtk_set_remove_all (self->selected); + } + for (pos = 0; pos < n; pos = start + n_items) { callback (pos, &start, &n_items, &in, data); @@ -223,10 +231,11 @@ gtk_multi_selection_add_or_remove (GtkSelectionModel *model, static gboolean gtk_multi_selection_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { - return gtk_multi_selection_add_or_remove (model, TRUE, callback, data); + return gtk_multi_selection_add_or_remove (model, unselect_rest, TRUE, callback, data); } static gboolean @@ -234,7 +243,7 @@ gtk_multi_selection_unselect_callback (GtkSelectionModel *model, GtkSelectionCallback callback, gpointer data) { - return gtk_multi_selection_add_or_remove (model, FALSE, callback, data); + return gtk_multi_selection_add_or_remove (model, FALSE, FALSE, callback, data); } static void diff --git a/gtk/gtkpropertyselection.c b/gtk/gtkpropertyselection.c index 948e75ae2d..3f4c192ba1 100644 --- a/gtk/gtkpropertyselection.c +++ b/gtk/gtkpropertyselection.c @@ -210,16 +210,24 @@ gtk_property_selection_unselect_all (GtkSelectionModel *model) static gboolean gtk_property_selection_add_or_remove (GtkSelectionModel *model, + gboolean unselect_rest, gboolean add, GtkSelectionCallback callback, gpointer data) { GtkPropertySelection *self = GTK_PROPERTY_SELECTION (model); - guint pos, start, n; + guint pos, start, n, n_items; gboolean in; guint min, max; guint i; + n_items = g_list_model_get_n_items (G_LIST_MODEL (self)); + if (unselect_rest) + { + for (i = 0; i < n_items; i++) + set_selected (self, i, FALSE); + } + min = G_MAXUINT; max = 0; @@ -241,7 +249,10 @@ gtk_property_selection_add_or_remove (GtkSelectionModel *model, } while (n > 0); - if (min <= max) + /* FIXME: do better here */ + if (unselect_rest) + gtk_selection_model_selection_changed (model, 0, n_items); + else if (min <= max) gtk_selection_model_selection_changed (model, min, max - min + 1); return TRUE; @@ -249,10 +260,11 @@ gtk_property_selection_add_or_remove (GtkSelectionModel *model, static gboolean gtk_property_selection_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { - return gtk_property_selection_add_or_remove (model, TRUE, callback, data); + return gtk_property_selection_add_or_remove (model, unselect_rest, TRUE, callback, data); } static gboolean @@ -260,7 +272,7 @@ gtk_property_selection_unselect_callback (GtkSelectionModel *model, GtkSelectionCallback callback, gpointer data) { - return gtk_property_selection_add_or_remove (model, FALSE, callback, data); + return gtk_property_selection_add_or_remove (model, FALSE, FALSE, callback, data); } static void diff --git a/gtk/gtkselectionmodel.c b/gtk/gtkselectionmodel.c index c2edaa2a51..1bd62e810d 100644 --- a/gtk/gtkselectionmodel.c +++ b/gtk/gtkselectionmodel.c @@ -115,6 +115,7 @@ gtk_selection_model_default_unselect_range (GtkSelectionModel *model, static gboolean gtk_selection_model_default_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { @@ -345,6 +346,7 @@ gtk_selection_model_unselect_all (GtkSelectionModel *model) /** * gtk_selection_model_select_callback: * @model: a #GtkSelectionModel + * @unselect_rest: whether previously selected items should be unselected * @callback: (scope call): a #GtkSelectionCallback to determine items to select * @data: data to pass to @callback * @@ -353,12 +355,13 @@ gtk_selection_model_unselect_all (GtkSelectionModel *model) */ gboolean gtk_selection_model_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data) { g_return_val_if_fail (GTK_IS_SELECTION_MODEL (model), FALSE); - return GTK_SELECTION_MODEL_GET_IFACE (model)->select_callback (model, callback, data); + return GTK_SELECTION_MODEL_GET_IFACE (model)->select_callback (model, unselect_rest, callback, data); } /** diff --git a/gtk/gtkselectionmodel.h b/gtk/gtkselectionmodel.h index 9a93a6a399..55a012e3d6 100644 --- a/gtk/gtkselectionmodel.h +++ b/gtk/gtkselectionmodel.h @@ -110,6 +110,7 @@ struct _GtkSelectionModelInterface gboolean (* select_all) (GtkSelectionModel *model); gboolean (* unselect_all) (GtkSelectionModel *model); gboolean (* select_callback) (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data); gboolean (* unselect_callback) (GtkSelectionModel *model, @@ -149,6 +150,7 @@ gboolean gtk_selection_model_unselect_all (GtkSelectionMod GDK_AVAILABLE_IN_ALL gboolean gtk_selection_model_select_callback (GtkSelectionModel *model, + gboolean unselect_rest, GtkSelectionCallback callback, gpointer data); GDK_AVAILABLE_IN_ALL diff --git a/testsuite/gtk/multiselection.c b/testsuite/gtk/multiselection.c index f56387a878..3c59d307f3 100644 --- a/testsuite/gtk/multiselection.c +++ b/testsuite/gtk/multiselection.c @@ -496,7 +496,7 @@ test_callback (void) assert_selection (selection, ""); assert_selection_changes (selection, ""); - ret = gtk_selection_model_select_callback (selection, select_some, data); + ret = gtk_selection_model_select_callback (selection, FALSE, select_some, data); g_assert_true (ret); assert_selection (selection, "3 4 5 7 8 9"); assert_selection_changes (selection, "2:7"); -- 2.30.2